-
Notifications
You must be signed in to change notification settings - Fork 12
Version 3 with cached cross chunk edges #454
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
pychunkedgraph/graph/cache.py
Outdated
| return cross_edges_decorated(node_id) | ||
|
|
||
| def parents_multiple(self, node_ids: np.ndarray, *, time_stamp: datetime = None): | ||
| node_ids = np.array(node_ids, dtype=NODE_ID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just saw this here (and some other places) - same as in #458: np.array will by default create a copy. np.asarray will avoid copies, if the requirements are already met.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good besides the one point - a tricky one though - that I marked
pychunkedgraph/graph/edits.py
Outdated
| new_cx_edges_d[layer] = edges | ||
| assert np.all(edges[:, 0] == new_id) | ||
| cg.cache.cross_chunk_edges_cache[new_id] = new_cx_edges_d | ||
| entries = _update_neighbor_cross_edges( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this here can introduce problems if a neighboring node is a neighbor to multiple new_l2_ids.
_update_neighbor_cross_edges looks right to me. It writes a complete new set of L2 edges for a node. But if the same node is updated multiple times, then only the last update is reflected. Maybe the logic here takes care of this somehow but then it still introduces multiple unnecessary writes.
So, if I am correct about this, the solution would be to consolidate this call across all new_l2_ids to only make one call per neighboring node id.
pychunkedgraph/graph/edits.py
Outdated
| new_cx_edges_d[layer] = edges | ||
| assert np.all(edges[:, 0] == new_id) | ||
| cg.cache.cross_chunk_edges_cache[new_id] = new_cx_edges_d | ||
| entries = _update_neighbor_cross_edges( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same issue as above
1ddb0a7 to
17dfc10
Compare
b13d8ec to
d90813d
Compare
c460a5a to
6a2c5da
Compare
bf90549 to
d2d9d44
Compare
280f9fe to
cc4cd46
Compare
6605bab to
dcbecd1
Compare
…ismatch (ingest); bump version
6648665 to
52708ab
Compare
…to reduce latency
MaxAgeGCRulefor previous column family with supervoxel cross chunk edges; only needed during ingest and they get deleted eventually.Summary of changes in
pychunkedgraph.ingest:layer>3untilrootlayer:This assumes all chunks at lower layer have been created before creating the current layer so we can no longer queue parent chunk jobs automatically when its children chunks are complete.
We must now ingest/create one layer at a time.
Summary of changes in
pychunkedgraph.graph.edits: